Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

fix: Expose ExplainEnum in the GQL schema #1204

Merged
merged 6 commits into from
Mar 21, 2023
Merged

fix: Expose ExplainEnum in the GQL schema #1204

merged 6 commits into from
Mar 21, 2023

Conversation

orpheuslummis
Copy link
Contributor

@orpheuslummis orpheuslummis commented Mar 20, 2023 •
edited
Loading

Relevant issue(s)

Resolves #1202

Description

Adds ExplainEnum to the GQL schema.
It fixes the introspection query error with Altair and GraphiQL.

Adds a test for the enum exist in resulting response schema . More general test cases should be preferred to this, such as general GraphQL schema validation.

(Discussion point?: It is a draft PR because I'm unsure whether to also change the schema integration test suite to handle graphql schema validation as a way to test 'large' schema responses from 'large' introspection queries.
Resolution: use assertContains to test the specific property, later introduce schema validation testing.)

Tasks

  • I made sure the code is well commented, particularly hard-to-understand areas.
  • I made sure the repository-held documentation is changed accordingly.
  • I made sure the pull request title adheres to the conventional commit style (the subset used in the project can be found in tools/configs/chglog/config.yml).
  • I made sure to discuss its limitations such as threats to validity, vulnerability to mistake and misuse, robustness to invalidation of assumptions, resource requirements, ...

How has this been tested?

Specify the platform(s) on which this was tested:

  • MacOS

@orpheuslummis orpheuslummis added area/schema Related to the schema system action/no-benchmark Skips the action that runs the benchmark. labels Mar 20, 2023
@orpheuslummis orpheuslummis added this to the DefraDB v0.5 milestone Mar 20, 2023
@orpheuslummis orpheuslummis self-assigned this Mar 20, 2023
@codecov
Copy link

codecov bot commented Mar 20, 2023 •
edited
Loading

Codecov Report

Merging #1204 (b07f650) into develop (0077635) will decrease coverage by 0.09%.
The diff coverage is 100.00%.

❗ Current head b07f650 differs from pull request most recent head 7fdd911. Consider uploading reports for the commit 7fdd911 to get more accurate results

Impacted file tree graph

@@             Coverage Diff             @@
##           develop    #1204      +/-   ##
===========================================
- Coverage    68.70%   68.62%   -0.09%     
===========================================
  Files          181      181              
  Lines        17123    17125       +2     
===========================================
- Hits         11765    11752      -13     
- Misses        4407     4417      +10     
- Partials       951      956       +5
Impacted Files Coverage Δ
request/graphql/schema/types/types.go 100.00% <ø> (ø)
request/graphql/schema/manager.go 97.36% <100.00%> (+0.04%) ⬆️

... and 5 files with indirect coverage changes

@orpheuslummis orpheuslummis marked this pull request as ready for review March 21, 2023 00:42
@orpheuslummis orpheuslummis requested a review from a team March 21, 2023 00:45
AndrewSisley
AndrewSisley requested changes Mar 21, 2023
View reviewed changes
Copy link
Contributor

@AndrewSisley AndrewSisley left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks Orpheus for find and sorting this! Prod code looks good, and thanks for adding the test - I do think that query needs to be trimmed down a bit before merge though.

request/graphql/schema/types/types.go Outdated Show resolved Hide resolved
request/graphql/schema/types/types.go Outdated Show resolved Hide resolved
tests/integration/schema/client_test.go Outdated Show resolved Hide resolved
tests/integration/schema/client_test.go Outdated Show resolved Hide resolved
@orpheuslummis orpheuslummis requested review from a team and AndrewSisley March 21, 2023 17:57
AndrewSisley
AndrewSisley approved these changes Mar 21, 2023
View reviewed changes
Copy link
Contributor

@AndrewSisley AndrewSisley left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks Orpheus!

@orpheuslummis orpheuslummis merged commit 8cf6855 into develop Mar 21, 2023
@orpheuslummis orpheuslummis deleted the orpheus/test/client-introspection branch March 21, 2023 18:16
shahzadlone pushed a commit that referenced this pull request Apr 13, 2023
@orpheuslummis @shahzadlone
fix: Expose ExplainEnum in the GQL schema (#1204)
1a5b4c2 
Adds ExplainEnum to the GQL schema. It fixes the introspection query error with Altair and GraphiQL.
Adds a test for the enum existence in resulting response schema.
shahzadlone pushed a commit to shahzadlone/defradb that referenced this pull request Feb 23, 2024
@orpheuslummis
fix: Expose ExplainEnum in the GQL schema (sourcenetwork#1204)
1b72037 
Adds ExplainEnum to the GQL schema. It fixes the introspection query error with Altair and GraphiQL.
Adds a test for the enum existence in resulting response schema.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@AndrewSisley AndrewSisley AndrewSisley approved these changes

Assignees

@orpheuslummis orpheuslummis

Labels
action/no-benchmark Skips the action that runs the benchmark. area/schema Related to the schema system
Projects
None yet
Milestone
DefraDB v0.5
Development

Successfully merging this pull request may close these issues.

ExplainType is not exposed as GraphQL default type
2 participants
@orpheuslummis @AndrewSisley